[Clang][Basic] Add `__has_feature` checks for CFI sanitizers #151348

moorabbit · 2025-07-30T14:59:51Z

Add __has_feature checks control flow integrity sanitizers.

github-actions · 2025-07-30T15:00:12Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-07-30T15:00:43Z

@llvm/pr-subscribers-clang

Author: None (moorabbit)

Changes

Support __has_feature(cfi) to check for control flow integrity sanitizers.

Fixes #151022

Full diff: https://github.com/llvm/llvm-project/pull/151348.diff

3 Files Affected:

(modified) clang/include/clang/Basic/Features.def (+1)
(modified) clang/include/clang/Basic/Sanitizers.def (+1-1)
(added) clang/test/Lexer/has_feature_cfi.c (+18)

diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def
index 72f23614aef11..4beb9a51dcf0b 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -303,6 +303,7 @@ FEATURE(is_trivially_assignable, LangOpts.CPlusPlus)
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
+FEATURE(cfi, LangOpts.Sanitize.hasOneOf(SanitizerKind::CFI))
 FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI))
 FEATURE(kcfi_arity, LangOpts.Sanitize.has(SanitizerKind::KCFI))
 FEATURE(modules, LangOpts.Modules)
diff --git a/clang/include/clang/Basic/Sanitizers.def b/clang/include/clang/Basic/Sanitizers.def
index 1d0e97cc7fb4c..aad758ceb9d40 100644
--- a/clang/include/clang/Basic/Sanitizers.def
+++ b/clang/include/clang/Basic/Sanitizers.def
@@ -134,7 +134,7 @@ SANITIZER("cfi-nvcall", CFINVCall)
 SANITIZER("cfi-vcall", CFIVCall)
 SANITIZER_GROUP("cfi", CFI,
                 CFIDerivedCast | CFIICall | CFIMFCall | CFIUnrelatedCast |
-                    CFINVCall | CFIVCall)
+                    CFINVCall | CFIVCall | CFICastStrict)
 
 // Kernel Control Flow Integrity
 SANITIZER("kcfi", KCFI)
diff --git a/clang/test/Lexer/has_feature_cfi.c b/clang/test/Lexer/has_feature_cfi.c
new file mode 100644
index 0000000000000..df472944e6d9e
--- /dev/null
+++ b/clang/test/Lexer/has_feature_cfi.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -E -fsanitize=cfi-cast-strict -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-derived-cast -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-icall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-mfcall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-unrelated-cast -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-nvcall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-vcall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E %s -o - | FileCheck --check-prefix=CHECK-NO-CFISAN %s
+
+#if __has_feature(cfi)
+int CFISanitizerEnabled();
+#else
+int CFISanitizerDisabled();
+#endif
+
+// CHECK-CFISAN: CFISanitizerEnabled
+// CHECK-NO-CFISAN: CFISanitizerDisabled
+

PiJoules

Thanks for taking this. Just a few comments from me.

PiJoules · 2025-07-30T16:33:59Z

clang/include/clang/Basic/Features.def

@@ -303,6 +303,7 @@ FEATURE(is_trivially_assignable, LangOpts.CPlusPlus)
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
+FEATURE(cfi, LangOpts.Sanitize.hasOneOf(SanitizerKind::CFI))


I think we'll also want to add || LangOpts.Sanitize.hasOneOf(SanitizerKind::LCFI) into this check.

I don't think SanitizerKind::LCFI exists as a variant of SanitizerKind. Did you mean SanitizerKind::KCFI?

Yup, my bad.

PiJoules · 2025-07-30T16:35:16Z

clang/test/Lexer/has_feature_cfi.c

+// RUN: %clang_cc1 -E -fsanitize=cfi-unrelated-cast -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-nvcall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E -fsanitize=cfi-vcall -o - %s | FileCheck --check-prefix=CHECK-CFISAN %s
+// RUN: %clang_cc1 -E %s -o - | FileCheck --check-prefix=CHECK-NO-CFISAN %s


Could you also add cases for -fsanitize=cfi, -fsanitize-cfi-cross-dso, and their removals with -fno-*?

Thanks for pointing that out!

Tried to address this in a new commit.
Let me know if there's any scenario I may have missed and that you would like included.

PiJoules

LGTM thanks

Support `__has_feature(cfi)` to check for control flow integrity sanitizers.

moorabbit · 2025-08-01T04:11:28Z

My apologies for the force-push. Issue with git.

CI was failing with the error missing sanitizer ignorelist. My last commit adds the -fno-sanitize-ignorelist flag to some clang invocations in order to fix the issue.

PiJoules · 2025-08-01T17:42:23Z

My apologies for the force-push. Issue with git.

CI was failing with the error missing sanitizer ignorelist. My last commit adds the -fno-sanitize-ignorelist flag to some clang invocations in order to fix the issue.

-fno-sanitize-ignorelist shouldn't be needed here. What errors were you originally seeing?

PiJoules · 2025-08-01T17:43:12Z

clang/test/Lexer/has_feature_cfi.c

+// RUN: %clang -E --target=x86_64-linux-gnu -fvisibility=hidden -fno-sanitize-ignorelist -fsanitize=cfi -fno-sanitize-cfi-cross-dso -fno-sanitize=cfi-nvcall,cfi-vcall,cfi-mfcall,cfi-icall -flto -c %s -o - | FileCheck %s --check-prefix=CHECK-CFISAN
+
+// Disable all CFI schemes. This essentially disables CFI sanitizers.
+// RUN: %clang -E --target=x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi -fno-sanitize-cfi-cross-dso -fno-sanitize=cfi-nvcall,cfi-vcall,cfi-mfcall,cfi-icall,cfi-cast-strict,cfi-derived-cast,cfi-unrelated-cast -flto -c %s -o - | FileCheck %s --check-prefix=CHECK-NO-CFISAN


If you're testing for x86_64-linux-gnu only, you'll probably want // REQUIRES: x86-registered-target also

On second thought, since I'm just running the preprocessor (i.e. clang -E), I probably don't need to set the target at all. I removed the target specification in the new commit. Let me know if you would like me to keep it and require x86-registered-target.

moorabbit · 2025-08-01T19:15:42Z

-fno-sanitize-ignorelist shouldn't be needed here. What errors were you originally seeing?

Here's the complete error message I get locally (CI shows the same):

clang: error: missing sanitizer ignorelist: '[...]/build/lib/clang/22/share/cfi_ignorelist.txt'

moorabbit · 2025-08-01T19:23:46Z

There was also a problem that caused a driver test to fail. I fixed it in b6a7ce7. The issue was that the driver was not expecting -fsanitize=cfi to also enable the cfi-cast-strict scheme.

After doing a little bit of digging, I found out that the original commit that added the CFI cast sanitizers didn't enable the cfi-cast-strict scheme by default (i.e. with -fsanitize=cfi). You need to explicitly specify -fsanitize=cfi-cast-strict in order to have it enabled. So I removed, in the last commit, CFICastStrict from the CFI sanitizer group.

Ran check-clang locally and didn't find any issues. CI should pass now.

pcc

cc @fmayer

We have been trying to break __has_feature(undefined_behavior_sanitizer) into fine grained checks (#148310). If we have __has_feature(cfi)mean CFI || KCFI that is sort of going in the opposite direction. Let's take some time to think about whether it is a good idea to combine them like this.

pcc · 2025-08-01T23:48:21Z

Also, I just learned that non-standard features are meant to be under __has_extension (EXTENSION macro) instead of __has_feature. See

llvm-project/clang/include/clang/Basic/Features.def

Line 20 in 154354e

// FEATURE(...) should be used to advertise support for standard language

fmayer · 2025-08-01T23:50:29Z

Also, I just learned that non-standard features are meant to be under __has_extension (EXTENSION macro) instead of __has_feature. See

llvm-project/clang/include/clang/Basic/Features.def

Line 20 in 154354e

// FEATURE(...) should be used to advertise support for standard language

Given kcfi already is in has_feature, maybe (if we decide to add this) it still is preferable to also put cfi there?

moorabbit · 2025-08-04T11:01:52Z

If we have __has_feature(cfi) mean CFI || KCFI that is sort of going in the opposite direction

Yes.

IMHO, it's a good idea to have a dedicated __has_feature for "regular" CFI, similar to what we have for KCFI.
Merging "regular" CFI and KCFI support under a single __has_feature(cfi) can be awkward, since it doesn’t definitively indicate support for regular CFI.

PiJoules · 2025-08-05T18:12:07Z

@pcc @fmayer How would y'all feel about proceeding with how UBSan handles this and instead introducing one __has_feature for each of the cfi schemes as opposed to one global one?

fmayer · 2025-08-05T19:20:59Z

Sounds good to me.

PiJoules · 2025-08-05T21:21:14Z

@moorabbit could you split the check into individual ones akin to #148310

PiJoules

LGTM but let's also wait for approval from either @pcc or @fmayer

PiJoules · 2025-08-06T17:37:06Z

clang/test/Lexer/has_feature_cfi.c

+// RUN: %clang -E -fvisibility=hidden -flto -fno-sanitize-ignorelist -fsanitize=cfi -fsanitize-cfi-cross-dso -c %s -o - | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -E -fvisibility=hidden -flto -fno-sanitize-ignorelist -fsanitize=cfi -fno-sanitize=cfi-nvcall,cfi-vcall,cfi-mfcall,cfi-icall -c %s -o - | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -E -fsanitize=kcfi -c %s -o - | FileCheck %s --check-prefix=CHECK-CFI
+// CHECK-CFI: CFISanitizerEnabled


Let's keep kcfi separated for now before we decide to combine them.

// CHECK-KCFI: KCFISanitizerEnabled

I agree to separate them, but I don't think we can change that later as soon as anyone uses that.

Sounds good. Tried to fix it in a new commit.

PiJoules · 2025-08-06T17:39:20Z

clang/include/clang/Basic/Features.def

@@ -303,6 +303,14 @@ FEATURE(is_trivially_assignable, LangOpts.CPlusPlus)
 FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
 FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
 FEATURE(is_union, LangOpts.CPlusPlus)
+FEATURE(cfi_sanitizer, LangOpts.Sanitize.hasOneOf(SanitizerKind::CFI | SanitizerKind::CFICastStrict | SanitizerKind::KCFI))


FEATURE(cfi_sanitizer, LangOpts.Sanitize.hasOneOf(SanitizerKind::CFI))

Fixed in a new commit. Thanks!

fmayer

lgtm thanks

small nit: I would say Add or Support rather than Enable in the first line of the commit message

moorabbit · 2025-08-07T11:42:53Z

Added a REQUIRES: target={{x86_64.*-linux.*}} in my last commit to fix the test failure on windows (-fsanitize=cfi-mfcall is not supported on windows) .
Since this PR is only adding a preprocessor feature and not something platform specific, I think it's fine to only test on x86_64-linux and skip other targets?

fmayer · 2025-08-07T21:18:13Z

Added a REQUIRES: target={{x86_64.-linux.}} in my last commit to fix the test failure on windows (-fsanitize=cfi-mfcall is not supported on windows) .
Since this PR is only adding a preprocessor feature and not something platform specific, I think it's fine to only test on x86_64-linux and skip other targets?

Yeah, I did the same with the UBSan change.

fmayer · 2025-08-08T19:51:01Z

Thanks!

github-actions · 2025-08-08T19:51:06Z

@moorabbit Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 30, 2025

PiJoules reviewed Jul 30, 2025

View reviewed changes

moorabbit requested a review from PiJoules July 31, 2025 10:52

PiJoules approved these changes Jul 31, 2025

View reviewed changes

moorabbit added 3 commits July 31, 2025 23:59

[Clang][Basic] Enable __has_feature(cfi)

49149a0

Support `__has_feature(cfi)` to check for control flow integrity sanitizers.

Add more test cases

75b0360

add -fno-sanitize-ignorelist to fix CI build issue

d741f65

moorabbit force-pushed the has-feature-cfi branch from edeef3c to d741f65 Compare August 1, 2025 04:02

moorabbit requested a review from PiJoules August 1, 2025 04:12

PiJoules reviewed Aug 1, 2025

View reviewed changes

Fix driver test failure

b6a7ce7

moorabbit requested a review from PiJoules August 1, 2025 19:26

PiJoules approved these changes Aug 1, 2025

View reviewed changes

pcc requested changes Aug 1, 2025

View reviewed changes

moorabbit requested a review from pcc August 4, 2025 16:53

PiJoules suggestion

7da5378

moorabbit requested a review from PiJoules August 6, 2025 03:13

PiJoules reviewed Aug 6, 2025

View reviewed changes

PiJoules suggestions

fded769

moorabbit requested review from PiJoules and fmayer August 6, 2025 19:18

fmayer approved these changes Aug 6, 2025

View reviewed changes

moorabbit changed the title ~~[Clang][Basic] Enable __has_feature(cfi)~~ [Clang][Basic] Add __has_feature checks for CFI sanitizers Aug 6, 2025

moorabbit added 2 commits August 7, 2025 00:09

Fix test failure on windows

cc13c4b

Specify the target platform the test is allowed to run on

64556aa

Merge branch 'main' into has-feature-cfi

f0094c0

Merge branch 'main' into has-feature-cfi

6e0c3c7

fmayer merged commit 6894abe into llvm:main Aug 8, 2025
9 checks passed

moorabbit deleted the has-feature-cfi branch August 9, 2025 11:01

[Clang][Basic] Add __has_feature checks for CFI sanitizers #151348

[Clang][Basic] Add __has_feature checks for CFI sanitizers #151348

Uh oh!

Conversation

moorabbit commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 30, 2025

Uh oh!

llvmbot commented Jul 30, 2025

Uh oh!

PiJoules left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PiJoules left a comment

Choose a reason for hiding this comment

Uh oh!

moorabbit commented Aug 1, 2025

Uh oh!

PiJoules commented Aug 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moorabbit commented Aug 1, 2025

Uh oh!

moorabbit commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcc left a comment

Choose a reason for hiding this comment

Uh oh!

pcc commented Aug 1, 2025

Uh oh!

fmayer commented Aug 1, 2025

Uh oh!

moorabbit commented Aug 4, 2025

Uh oh!

PiJoules commented Aug 5, 2025

Uh oh!

fmayer commented Aug 5, 2025

Uh oh!

PiJoules commented Aug 5, 2025

Uh oh!

PiJoules left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmayer left a comment

Choose a reason for hiding this comment

Uh oh!

moorabbit commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmayer commented Aug 7, 2025

Uh oh!

Uh oh!

fmayer commented Aug 8, 2025

Uh oh!

github-actions bot commented Aug 8, 2025

[Clang][Basic] Add `__has_feature` checks for CFI sanitizers #151348

[Clang][Basic] Add `__has_feature` checks for CFI sanitizers #151348

moorabbit commented Jul 30, 2025 •

edited

Loading

moorabbit commented Aug 1, 2025 •

edited

Loading

moorabbit commented Aug 7, 2025 •

edited

Loading